feat: add 'Next' button and next execution handling for templates#104
feat: add 'Next' button and next execution handling for templates#104jcalado wants to merge 1 commit intonxtedition:masterfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements a "next" command functionality for CasparCG graphics templates. The feature allows triggering a next action on templates both individually and in bulk, enabling better control over template transitions.
- Adds
nextCountstate tracking in the graphics-kit TemplateProvider - Implements
caspar-nextandcaspar-next-allactions in the client reducer - Adds UI button and keyboard shortcut ('n') to trigger next actions
- Integrates server-side support for the CasparCG
NEXTcommand
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/graphics-kit/src/use-caspar.js | Exposes nextCount from context to template hooks |
| packages/graphics-kit/src/TemplateProvider.jsx | Adds nextCount state and window.next() handler |
| packages/caspar-graphics/src/node/server.js | Implements server-side CasparCG NEXT command |
| packages/caspar-graphics/src/client/app.jsx | Adds reducer actions and keyboard shortcut for next functionality |
| packages/caspar-graphics/src/client/TemplatePreview.jsx | Triggers next on templates via nextExecutionId changes |
| packages/caspar-graphics/src/client/Sidebar.jsx | Adds "Next" button in template controls |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const updateTemplate = (data, idx = index) => { | ||
| const templates = [...state.templates] | ||
| templates[index] = { ...template, ...data } | ||
| templates[idx] = { ...state.templates[idx], ...data } | ||
| return { ...state, templates } | ||
| } |
There was a problem hiding this comment.
The updateTemplate function can be called with an invalid index when no template is found. The removed check for index === -1 means that updateTemplate will attempt to access state.templates[-1] which returns undefined, causing the spread operation to fail. Add a guard check before the switch statement to return early if index === -1 for single-template actions.
| const templates = state.templates.map(t => | ||
| t.enabled && t.show ? { ...t, nextExecutionId: Date.now() } : t |
There was a problem hiding this comment.
Multiple templates triggered simultaneously with caspar-next-all will receive the same Date.now() timestamp. If the execution is fast enough, these identical timestamps may not trigger the useEffect dependencies properly for all templates. Consider using a more robust approach like incrementing unique IDs (e.g., Date.now() + index) to ensure each template gets a unique value.
| const templates = state.templates.map(t => | |
| t.enabled && t.show ? { ...t, nextExecutionId: Date.now() } : t | |
| const templates = state.templates.map((t, idx) => | |
| t.enabled && t.show ? { ...t, nextExecutionId: Date.now() + idx } : t |
No description provided.